Hello @adrianf0 ,

There are 2 ways of going for something like this.

Write file list at CMake configure phase (unused files might be present)

This approach is much simpler and it works for any file type, not just SV.
But the disadvantage is that it includes files that might not be used in the hierarchy.

Generate at build phase, with analyzing the hierarchy.

In case this is needed, I support the decision to go for slang in place of vhier.
I used slang for other things and its a really good piece of software.

When it comes to the pull request, I have few comments:

1. The python wrapper doesn't seem to be doing anything? Why not call Slang directory from Make/Ninja target?
2. We are making assumptions for -DSYNTHESIS flag, although this might work you, in other projects it can be called SYNTH, or it might not even exist.
3. Typically its better to find executable with find_program(slang) as it leaves much more flexibility to conform to different development environments. Makes it also possible to override the executable without modifying the environment variables.
4. It would be better to use custom_command/custom_target combination, as when you use custom_target only, this target will always run, while in case of custom_command it runs only when input files change, so no unnecessary generation. This is much more valuable when output of the command is fed to another command, where a lot of redundant rebuilding can be avoided by properly tracking the file changes.
5. Usage of ALL for the target, this can be a bit intrusive to the build flow, and not having a way to opt-out is a problem.

I think this probably will work well for your project, but it has some issues with not being generic enough because of assumptions that are made.

Just another comment,

we should probably rename it from generate_sources_list to something that suggest that it generates SystemVerilog/Verilog sources filtered.

I don't have yet suggestion for a good name, will think about it.

Just chiming in about the -DSYNTHESIS: this is defined automatically by some tools (e.g. Genus).

1. The python wrapper doesn't seem to be doing anything? Why not call Slang directory from Make/Ninja target?

2. We are making assumptions for -DSYNTHESIS flag, although this might work you, in other projects it can be called SYNTH, or it might not even exist.

3. Typically its better to find executable with `find_program(slang)` as it leaves much more flexibility to conform to different development environments. Makes it also possible to override the executable without modifying the environment variables.

4. It would be better to use custom_command/custom_target combination, as when you use custom_target only, this target will always run, while in case of custom_command it runs only when input files change, so no unnecessary generation. This is much more valuable when output of the command is fed to another command, where a lot of redundant rebuilding can be avoided by properly tracking the file changes.

5. Usage of ALL for the target, this can be a bit intrusive to the build flow, and not having a way to opt-out is a problem.

@Risto97 Thanks a lot for your review.

1. Python wrapper: The Python wrapper is now removed. The CMake function directly calls slang as you recommended, making the process more transparent and easier to override.

2. -DSYNTHESIS flag: It is a common convention in RTL (Register Transfer Level) design to use a SYNTHESIS macro to mark synthesizable code. This allows designers to distinguish between code that should be included during synthesis and code that is only relevant for simulation, testbenches, etc. As @mksoc mentioned, the synthesis tools, like genus, set this macro during synthesis.

3. find_program(slang): The code uses find_program(SLANG_EXECUTABLE slang) to locate the slang executable, so it can be overridden via CMake variables or environment, as you suggested.

4. custom_command/custom_target: The build flow now uses add_custom_command to generate output files only when input files change, and add_custom_target to provide a named target. This avoids unnecessary regeneration and redundant rebuilding.

5. ALL target: The custom target is no longer added to ALL. Users must explicitly build the <ip>_source_list target, so it is not intrusive and can be opted into.

Hello @adrianf0 ,

Thank you for making the changes.

-DSYNTHESIS flag: It is a common convention in RTL (Register Transfer Level) design to use a SYNTHESIS macro to mark synthesizable code. This allows designers to distinguish between code that should be included during synthesis and code that is only relevant for simulation, testbenches, etc. As @mksoc mentioned, the synthesis tools, like genus, set this macro during synthesis.

I feel like this tool can be used for other purposes than generating file lists for synthesis.
It can be used for FPGA, SIMULATION, or just filtering files based on a set -DDEFINES.

In that sense I dont think its necessary to have an explicit argument SYNTHESIS in the CMake function.
This can easily be achieved in 2 ways:

add_ip(mcu)

ip_compile_definitions(mcu SYSTEMVERILOG -DSYNTHESIS=1)

generate_sources_list(mcu)

or add a ARGS or DEFINES argument to generate_sources_list function and then:

add_ip(mcu)

generate_sources_list(mcu ARGS -DSIMULATION=1)

You can see that this way we have something more flexible that does not limit us to just passing one predefined macro to slang

or add a ARGS or DEFINES argument to generate_sources_list function and then:

add_ip(mcu)

generate_sources_list(mcu ARGS -DSIMULATION=1)

@Risto97 Thank you for your feedback and for outlining the two approaches.

The second option you described, adding an argument to generate_sources_list for passing arbitrary macros or arguments has now been implemented. The function now accepts a SLANG_ARGS keyword argument, which allows you to pass any set of defines or options directly to the slang tool.

For example, you can now use:

add_ip(mcu)
generate_sources_list(mcu SLANG_ARGS -DSYNTHESIS)